Skip to content

Conversation

@gr5
Copy link
Collaborator

@gr5 gr5 commented May 14, 2025

No description provided.

…rking better and added knife plot. May not be completely accurate yet. More testing in progress. but this save current state.
… when input wave length is 550. Added hot keys I and ctrl-o. Limited profile y offset to just one wave from when multiple are selected. Change percent control to a button.
…y help. Added auto collimation function to ronchi and foucault page.
…ectories. Added user settable prefixes to wave front movie generation.
… for menus. Updated some help and tool tips. Added L hot key to open igram directory.
…g back from auto collimation in Foucault view.
…rredtly and astig and wave front movies can both be made.
…age 3 of the report. Added log display to the process wizard window and various QApplication::processevents so the screen updates the displays during the process for better user feedback.
…ile can be moved up or down in y value on profile plot. Letting any profile be selected to be moved.
…ot headings to be sort enabled. Disabled ability to edit polar plot table entries.
@gr5 gr5 requested a review from githubdoe May 14, 2025 12:58
@atsju
Copy link
Collaborator

atsju commented May 14, 2025

That's a lot of changes. Kindly let me fix the CI scripts and do a quick code review before merging and releasing (release cannot work without script anyway). Might take until this week end as I'm quite busy right now.

In the meantime, you can update the releaseNotes file

@atsju
Copy link
Collaborator

atsju commented May 14, 2025

If you have some time, can you look at the newly introduced build warnings ? : https://github.com/githubdoe/DFTFringe/actions/runs/15024125124

They are not particularly bad, but they may mask really bad warnings so they need to be gone.

edit: maybe not newly introduced

@gr5
Copy link
Collaborator Author

gr5 commented May 14, 2025

They are not particularly bad, but they may mask really bad warnings so they need to be gone.

I looked at them all. Well what I mean is I compiled it myself in the qtcreator ide and got 29 warnings. I looked at all 29. There may be one that you linked to that isn't showing up with compiler settings I'm using.

Anyway nothing serious. I could fix most of them quite quickly I admit. I don't think I want to at this point. When I'm building something and I see warnings, I fix them but I only notice them if they are happening on the files I'm editing (because only those ones get built).

And Dale seems to be ignoring them anyway. So I don't plan to fix them at this time.

@githubdoe githubdoe merged commit 292b059 into master May 14, 2025
8 checks passed
Copy link
Collaborator

@atsju atsju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the PR is too long to be review-able.
I will to look at the 3 comments I mentioned. Please delay the release just a little bit.


// Set the logging format
spdlog::get("logger")->set_pattern("[%Y-%m-%d %H:%M:%S.%e] [%^%l%$] %v");
//spdlog::get("logger")->set_pattern("[%Y-%m-%d %H:%M:%S.%e] [%^%l%$] %v");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this must be uncommented in release

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry it was part of my attempt to disable that for my debugging. I don't remember but maybe the ifndef technique I used for part of that did not work there for reasons I did not understand nor could get to work. IIRC

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had noticed this but when I looked at the log output there was date and time included. Maybe I should have looked in the log file? I didn't think to.

}
catch(std::exception& e) {

qDebug() << "Exception writing video " << e.what();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably be qWarning (better for logging purpose)

Comment on lines +90 to +93
//For some reason the original starter code of makeChart was in this function but it caused a crash.
// I could never figure out why because that code was taken from a working Qt example. When
// moved to inside another function it worked. So there you go. Exact same code with no changes worked there but not here.
makeChart();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this makes me super uncomfortable. Might just break later on if we don't understand it.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me too but I did a lot of debugging that I did not list in the comment. I narrowed the issue down to the line where it attaches the radial axis to the plot. At that point there is no debug code to show what happens inside that call. Like I said that code works everywhere else except in that routine. At that point I had to punt. Others are welcome to dig deeper.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested on linux and got no issue. I will try the linux build also. So far I did NOT reproduce your issue.

@gr5
Copy link
Collaborator Author

gr5 commented May 15, 2025

I spent an hour testing. No problems so far. Want to test a few more things.

@gr5
Copy link
Collaborator Author

gr5 commented May 15, 2025

I don't see any log file at all 😢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants